Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue #15 makeAssimilatePrototype invokes getters when copying me… #16

Closed
wants to merge 1 commit into from

Conversation

timnew
Copy link

@timnew timnew commented May 14, 2015

Well, it seems this approach solved the issue

@timnew
Copy link
Author

timnew commented May 14, 2015

This pull request is intended to fix gaearon/react-hot-loader#131

@gaearon
Copy link
Owner

gaearon commented May 16, 2015

Thanks! I have a busy week but I'll come around to check this out in a week or so.

@jhollingworth
Copy link

👍 I've just seeing the same issue

@jquense
Copy link

jquense commented Jul 21, 2015

not trying to be that guy, but any chance we will see this merged and released? I need to turn off hot loading otherwise

@benmosher
Copy link

Just replaced npm's tip version of react-hot-api with this and my local react-hot-loader is working with my custom @bind-decorated methods (replaces method with a getter that binds on the way out, like a forced ::this.whatever).

AFAIK, this is PR is a more generally correct method of transferring own-properties. 👍

@gaearon
Copy link
Owner

gaearon commented Aug 5, 2015

not trying to be that guy, but any chance we will see this merged and released? I need to turn off hot loading otherwise

I don't have time right now. I'm focused on getting Redux 1.0 out. I don't want to release a fix I have not tested extensively because it might break other people's stuff, and then I'd have to spend even more time when I don't have any.

I will get back to improving React Hot Loader later this month, and then I'll make sure to merge this and other important fixes. Please have some more patience. :-)

@benmosher
Copy link

FYI, folks can use this in the interim by:

  • removing react-hot-loader (npm r react-hot-loader --save-dev)
  • install timnew/react-hot-api (npm i timnew/react-hot-api --save-dev)
  • then install react-hot-loader 1.2.7* (npm i react-hot-loader@1.2.7 --save-dev)

This is working for me, but I'm pretty new to React hot loading so I'm not sure what could go wrong. 😅

*1.2.8 is not compatible with react-hot-api 1.4.4 and would thus install its own version in its node_modules.

@gaearon
Copy link
Owner

gaearon commented Aug 6, 2015

Thanks for providing interim instrutions @benmosher!

@gaearon
Copy link
Owner

gaearon commented Aug 21, 2015

This is fixed in React Proxy in a slightly more accurate way.
I'll soon migrate RHA to use it, stay tuned.

Thanks for the PR!

@gaearon gaearon closed this Aug 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants